fix: multiple background tasks make outbound http re... in...#9021
Open
orbisai0security wants to merge 1 commit intomakeplane:previewfrom
Open
fix: multiple background tasks make outbound http re... in...#9021orbisai0security wants to merge 1 commit intomakeplane:previewfrom
orbisai0security wants to merge 1 commit intomakeplane:previewfrom
Conversation
Automated security fix generated by Orbis Security AI
orbisai0security
requested review from
dheeru0198 and
pablohashescobar
as code owners
|
|
Contributor
📝 WalkthroughWalkthroughAdded explicit ChangesWebhook URL Validation Error Handling
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/plane/bgtasks/webhook_task.py (1)
348-351: ⚡ Quick winPersist blocked webhook attempts in
save_webhook_log.Line 348 returns early on validation failure, but this path currently skips persistent webhook logs. Adding a blocked-entry log keeps security blocks visible in the same audit stream as success/failure deliveries.
Proposed patch
except ValueError as e: # URL failed SSRF/IP-range validation — do not send or retry - logger.error(f"Webhook {webhook.id} blocked: URL validation failed: {e}") + reason = f"URL validation failed: {e}" + save_webhook_log( + webhook=webhook, + request_method=action, + request_headers=headers, + request_body=payload, + response_status=403, + response_headers="", + response_body=reason, + retry_count=self.request.retries, + event_type=event, + ) + logger.error(f"Webhook {webhook.id} blocked: {reason}") return🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/plane/bgtasks/webhook_task.py` around lines 348 - 351, The early return in the except ValueError block for SSRF/IP-range validation prevents persisting the blocked attempt; before returning from that block in webhook_task.py (where logger.error logs "Webhook {webhook.id} blocked..."), call the existing save_webhook_log function to persist a blocked/validation-failed entry for this webhook (include webhook.id, the request URL or target, the error message e, and a delivery status like "blocked" or "validation_failed"), then return; this ensures blocked deliveries appear in the same audit stream as successes/failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/api/plane/bgtasks/webhook_task.py`:
- Around line 348-351: The early return in the except ValueError block for
SSRF/IP-range validation prevents persisting the blocked attempt; before
returning from that block in webhook_task.py (where logger.error logs "Webhook
{webhook.id} blocked..."), call the existing save_webhook_log function to
persist a blocked/validation-failed entry for this webhook (include webhook.id,
the request URL or target, the error message e, and a delivery status like
"blocked" or "validation_failed"), then return; this ensures blocked deliveries
appear in the same audit stream as successes/failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5a36e423-1003-4fb3-94c7-4b1dbecb4b5c
📒 Files selected for processing (1)
apps/api/plane/bgtasks/webhook_task.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix high severity security issue in
apps/api/plane/bgtasks/webhook_task.py.Vulnerability
V-003apps/api/plane/bgtasks/webhook_task.py:333Description: Multiple background tasks make outbound HTTP requests to URLs that may be user-supplied without validating that the destination is a legitimate external host. In webhook_task.py:333, the webhook.url value is user-configured and passed directly to requests.post() with no IP range or hostname validation. In work_item_link_task.py, URLs from work item links are fetched without restriction. An attacker can supply URLs pointing to internal services such as the AWS EC2 instance metadata endpoint (http://169.254.169.254/latest/meta-data/), internal Redis instances, or other private network services to exfiltrate credentials or interact with infrastructure not intended to be publicly accessible.
Changes
apps/api/plane/bgtasks/webhook_task.pyVerification
Automated security fix by OrbisAI Security
Summary by CodeRabbit